Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alter font loading code to support fallback fonts in fontFamily #31

Merged
merged 8 commits into from
Nov 15, 2022

Conversation

stevenpetryk
Copy link

@stevenpetryk stevenpetryk commented Nov 14, 2022

This is some pre-work to improving how non-latin scripts render. On the web, one can declare fallback fonts easily:

font-family: "Comic Sans MS", "Noto Sans", system-ui;

And that'll attempt to use those fonts, in that order, falling back to the next font whenever a character can't be rendered by the given font.

React Native doesn't support this, and issues asking for support have been closed as stale by a bot. This adds some sliiiightly jank code to support it. I originally wanted to do something like this:

// ❗️ (just an example, not what this PR actually enables)
const styles = StyleSheet.create({
  fontFamily: ['Font One', 'Font Two', 'Font Three', 'sans-serif']
})

But I settled for this to minimize the diff size compared to upstream:

// ✅ what this PR actually enables (not as cool, but easier to implement)
const styles = StyleSheet.create({
  fontFamily: 'Font One, Font Two, Font Three'
})

The native code now just splits fontFamily names by commas and uses that to construct fallback-filled UIFont/Typeface instances.

@stevenpetryk stevenpetryk changed the base branch from master to 0.68-discord-1 November 14, 2022 18:57
@stevenpetryk stevenpetryk force-pushed the i18n-fallback-testing branch 3 times, most recently from ec024e0 to 767848d Compare November 14, 2022 18:59
@@ -192,14 +192,50 @@ - (NSParagraphStyle *)effectiveParagraphStyle

- (UIFont *)effectiveFont
{
// FIXME: RCTFont has thread-safety issues and must be rewritten.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

confidence inspiring

Comment on lines 143 to 155
String[] fontFamilyNames = fontFamilyName != null ? fontFamilyName.split(",") : null;
if (fontFamilyNames != null) {
for (int i = 0; i < fontFamilyNames.length; i++) {
fontFamilyNames[i] = fontFamilyNames[i].trim();
}
}
if (fontFamilyNames != null && fontFamilyNames.length > 1) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
return createAssetTypefaceWithFallbacks(fontFamilyNames, style, assetManager);
} else {
fontFamilyName = fontFamilyNames[0];
}
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few checks happening here:

  1. First we really carefully try to see whether a comma-separated list was passed to fontFamilyName
  2. If it was, we split it and trim whitespace off each entry (to allow folks to put spaces around commas)
  3. Finally, iff that list has more than one thing and we're on a high enough version of Android, we use the fallback font creation code. If we're on an older version of Android, we just ignore the fallbacks and proceed wit the original logic.

@stevenpetryk stevenpetryk marked this pull request as ready for review November 15, 2022 18:33
@discord discord deleted a comment from github-actions bot Nov 15, 2022
@stevenpetryk stevenpetryk changed the title i18n fallback testing Alter font loading code to support comma-separated values for fontFamily Nov 15, 2022
@stevenpetryk stevenpetryk changed the title Alter font loading code to support comma-separated values for fontFamily Alter font loading code to support fallback fonts in fontFamily Nov 15, 2022
Comment on lines 110 to 115
private boolean getIsReduceMotionEnabledValue() {
float defaultAnimationScale = Float.parseFloat(Settings.Global.TRANSITION_ANIMATION_SCALE);
float animationScale = Settings.Global.getFloat(mContentResolver, defaultAnimationScale);
return animationScale == 0f;
String value =
Settings.Global.getString(mContentResolver, Settings.Global.TRANSITION_ANIMATION_SCALE);

return value != null && value.equals("0.0");
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes are unrelated to the PR but they're necessary to get RN to compile locally

@stevenpetryk stevenpetryk merged commit 5e1d210 into 0.68-discord-1 Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant